fix: Set Substrait output_type on aggregate functions - #25090

@alamb could a committer approve the workflows on this PR and on #25091? Both were opened on 2026-09-08 and neither has had a single check run since. The one green mark on each is the Process labeler — Rust, Dev, CodeQL, Dependencies, Detect breaking changes and Large files PR check are all sitting at action_required.

I asked on #25191 a few hours ago rather than pinging the same reviewers repeatedly, so I'm folding these two into one request here instead of opening a third thread.

Both are rebased onto current main, no conflicts, one commit each. On the rebased commits, cargo test -p datafusion-substrait passes on both — 272 tests, 0 failures.

They are small and independent:

• This PR (fix: Set Substrait output_type on aggregate functions #25090, closes Substrait producer omits the required output_type on aggregate functions #25049) — one file, +53/−3. Sets AggregateFunction.output_type from the logical expression's field, the same path scalar functions already use.
• fix: Derive Substrait intersection nullability from every input #25091 (closes Substrait intersection schemas retain primary-input nullability #25042) — narrows intersection output nullability to the AND of the inputs, per the Substrait set-operation rules.

Happy to rework either if you'd prefer a different approach. And if approving workflows on fork PRs is itself part of what #25148 is trying to reduce, I'm fine with these waiting until there's a decision there — just wanted to flag that they aren't stalled on anything I can fix from my side.

Codecov Report

❌ Patch coverage is 79.31034% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.93%. Comparing base (f8cc678) to head (3616533).

@@            Coverage Diff             @@
##             main   #25090      +/-   ##
==========================================
- Coverage   81.93%   81.93%   -0.01%     
==========================================
  Files        1133     1133              
  Lines      423529   423557      +28     
  Branches   423529   423557      +28     
==========================================
+ Hits       347028   347049      +21     
- Misses      55910    55912       +2     
- Partials    20591    20596       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Fair question, and my PR description was the reason for it — it argued from the spec and only said a
consumer "can reject such plans", without naming one. Here is the concrete failure, measured rather
than asserted.

The engine is substrait-java (io.substrait:core:0.103.0). It reads the field unconditionally:

// ProtoAggregateFunctionConverter.java:82
.outputType(protoTypeConverter.from(measure.getOutputType()))

// ProtoTypeConverter.java:125
case KIND_NOT_SET:
  throw new UnsupportedOperationException("Type is not set: " + type);

There is no hasOutputType() guard on that path, so every aggregate measure we export hits it.

Reproduction, with no DataFusion consumer in the loop. I built SELECT sum(i) FROM t over one
non-null i64 column, called to_substrait_plan, wrote the protobuf to a file, and handed that file
to substrait-java's ProtoPlanConverter.

One wrinkle that matters if you try this, and that I should have led with: a DataFusion plan as
emitted today does not even reach the output_type check. It fails earlier with

IllegalStateException: Function 'sum' references URN anchor -1, but no URN is registered at that anchor

because we still write extension_urn_reference: u32::MAX for every function declaration
(extensions.rs:120), which is the known gap tracked in #11545. To isolate this bug I registered
extension:io.substrait:functions_arithmetic on the plan and used the compound key sum:i64. I
changed nothing else in either plan.

So output_type is a second, independent gate sitting behind #11545. Fixing this alone does not
make DataFusion plans readable by substrait-java. Leaving it unfixed means they still will not be
once #11545 is. The type substrait-java recovers, nullable i64, is exactly what the optimized
DataFusion plan's own schema says sum(t.i) is, so the value written is right and not merely
present.

Why nothing in our suite catches it. Our consumer never reads this field — the only output_type
read anywhere under logical_plan/consumer/ is in cast.rs. Producer and consumer therefore agree
on a shape the spec does not describe, and every DataFusion-to-DataFusion round trip passes.

Spec wording, identical in the pinned substrait 0.63.0 crate, in upstream v0.87.0 and on
main:

Must be set to the return type of the function, exactly as derived using the declaration in the
extension.

Precedent. This is the same defect and the same fix as #15831, fixed by #20597, which set
output_type from Expr::ScalarFunction(..).to_field(schema) for scalar functions. This PR uses
that identical path for aggregates.

One correction to my own description while I am here: it lists from_in_list as a remaining
follow-up site, which is wrong. from_in_list emits SingularOrList, which has no output_type
field at all. The sites that genuinely still omit it are window functions
(window_function.rs:110), the LIKE and NOT LIKE paths (scalar_function.rs:296 and :312), and
the not() wrapper in utils.rs:106. I have fixed that sentence in the description.

I will answer the same question on #25100 and #25190 on their own threads. One clarification about
#25100, since I had it half wrong when I first wrote this: substrait-java's core reader does not
throw on AGGREGATION_PHASE_UNSPECIFIED, but its Spark converter maps it to Spark's Final
(ToAggregateFunction.scala:77, their own comment: UNSPECIFIED implies INTERMEDIATE_TO_RESULT),
while INITIAL_TO_RESULT maps to Complete. So that one is not merely a spec argument either — it
is a silent misreading rather than an exception, which is arguably the worse failure of the two.